Skip to content

ref(batcher): Only flush the bucket that triggered the flush event#6168

Open
sentrivana wants to merge 23 commits intomasterfrom
ivana/batcher-flush-by-bucket
Open

ref(batcher): Only flush the bucket that triggered the flush event#6168
sentrivana wants to merge 23 commits intomasterfrom
ivana/batcher-flush-by-bucket

Conversation

@sentrivana
Copy link
Copy Markdown
Contributor

@sentrivana sentrivana commented Apr 29, 2026

Description

The span batcher uses multiple buffers (buckets) to store spans, keyed by trace_id. If a bucket fills up, we flush the whole buffer (all the buckets) instead of just the one bucket that actually should be flushed. This leads to too-frequent, busy flushes and is also against the spec.

With this change, we keep a set of buckets to flush. We still support full flushes for cases when sentry_sdk.flush() is called manually, or when the SDK is shutting down. Time-based flushes are also still global, i.e., not per bucket.

Issues

Reminders

@linear-code
Copy link
Copy Markdown

linear-code Bot commented Apr 29, 2026

@sentrivana sentrivana changed the title ref(batcher): Only flush the bucket that triggered the limit ref(batcher): Only flush bucket that triggered the flush event Apr 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

Codecov Results 📊

13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 8.04s

All tests are passing successfully.

✅ Patch coverage is 85.71%. Project has 14946 uncovered lines.

Files with missing lines (1)
File Patch % Lines
_span_batcher.py 82.93% ⚠️ 21 Missing and 11 partials

Generated by Codecov Action

Comment thread sentry_sdk/_span_batcher.py Outdated
Comment thread sentry_sdk/_span_batcher.py
Comment thread sentry_sdk/_span_batcher.py Outdated
Comment thread sentry_sdk/_span_batcher.py Outdated
Comment thread sentry_sdk/_span_batcher.py
assert get_profiler_id() is None, "profiler should have stopped"


def test_transport_format(sentry_init, capture_envelopes):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one was split in two (test_default_attributes and test_transport_format) and the latter was moved to test_span_buffer

@sentrivana sentrivana changed the title ref(batcher): Only flush bucket that triggered the flush event ref(batcher): Only flush the bucket that triggered the flush event Apr 30, 2026
Comment thread sentry_sdk/_span_batcher.py Outdated
Comment thread sentry_sdk/_span_batcher.py Outdated
@sentrivana sentrivana marked this pull request as ready for review April 30, 2026 11:55
@sentrivana sentrivana requested a review from a team as a code owner April 30, 2026 11:55
Comment thread sentry_sdk/_span_batcher.py
@sentrivana sentrivana marked this pull request as draft April 30, 2026 11:59
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2b37852. Configure here.

Comment thread sentry_sdk/_span_batcher.py
@sentrivana sentrivana marked this pull request as ready for review April 30, 2026 12:22
Copy link
Copy Markdown
Contributor

@alexander-alderman-webb alexander-alderman-webb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
It's probably not great having so many sleeps in the test, but the alternative is relying on private details (which wouldn't be good either).

Comment thread tests/tracing/test_span_buffer.py Outdated
@sentrivana
Copy link
Copy Markdown
Contributor Author

It's probably not great having so many sleeps in the test, but the alternative is relying on private details (which wouldn't be good either).

Yeah definitely. Tried to keep them as low as possible at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only flush trace buckets that triggered a flush event

2 participants